Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating the README and templates #501

Merged
merged 32 commits into from
Sep 29, 2023
Merged

Updating the README and templates #501

merged 32 commits into from
Sep 29, 2023

Conversation

erinyoung
Copy link
Contributor

@erinyoung erinyoung commented Dec 5, 2022

UPDATE : THIS PR IS READY TO REVIEW!!!

It's always good to go through the documentation of a repo every once and awhile. Most things are not changed, but I do have some suggestions.

@erinyoung
Copy link
Contributor Author

And... these files trigger the github actions workflow, so I need to change the file paths.

@kapsakcj
Copy link
Collaborator

kapsakcj commented Jan 31, 2023

OK, I'm happy with the way this PR is at this point, but another set of eyes on the rendered markdown & dockerfile templates would be good.

I made some small edits to your changes for clarity & cleaned up some of the links & markdown formatting. I use VSCode to edit Markdown and now I get all kinds of warnings like how bullet points need to be surrounded above and below by empty lines

I also added a template dockerfile with micromamba as the base image

obviously the github actions has failed and I'm fine to override and merge the PR

Apologies that it took me this long to review and do my part!

@kapsakcj
Copy link
Collaborator

also forgot to say that the mermaid diagram is awesome, thanks for adding that! Super helpful visual

@erinyoung erinyoung marked this pull request as ready for review July 18, 2023 18:00
@erinyoung
Copy link
Contributor Author

erinyoung commented Jul 18, 2023

This PR is now ready to review!!!

This is going to fail the GA tests because the Dockerfile template isn't functional - so ignore the GA tests.

This PR is to update and double check the documentation of the StaPH-B/docker-builds repo. This is just to ensure the language and examples are in harmony with current recommended practices.

In the dockerfile-template directory there are three dockerfiles to use for examples:

  • Dockerfile (which causes GA to crash)
  • Dockerfile_builder - which shows adding an additional stage prior to 'app'
  • Dockerfile_mamba - which shows using mamba to install something - thank you @kapsakcj !!!!
  • README.md - as a template for a tool-specific readme

In the docker directory there are several files that are used to help those creating Dockerfiles. I only have suggested edits for a subset of docs.

  • contribute.md
  • get_containers.md * no edits
  • make_containers.md * no edits
  • run_containers.md
  • useful_links.md * no edits

@kapsakcj kapsakcj self-requested a review July 26, 2023 15:25
@erinyoung erinyoung mentioned this pull request Sep 6, 2023
9 tasks
@erinyoung
Copy link
Contributor Author

I added in a python-centric Dockerfile template for those tools that are installed via pip.

I am not going to add any additional files to his PR because I would like this PR merged before the docker training in October.

Copy link
Collaborator

@kapsakcj kapsakcj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good, thanks for adding the additional python dockerfile template!

@kapsakcj kapsakcj merged commit 371e737 into StaPH-B:master Sep 29, 2023
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants